Skip to content

Conversation

@EndlessJour9527
Copy link
Collaborator

implement vertexAttribI4i, vertexAttribI4ui, vertexAttrbI4iv, and vertexAttribI4uiv methods with argument validation

…bI4iv, and vertexAttribI4uiv methods with argument validation
Copilot AI review requested due to automatic review settings October 28, 2025 07:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements four WebGL2 vertex attribute methods that were previously marked as "Not implemented": vertexAttribI4i, vertexAttribI4ui, vertexAttribI4iv, and vertexAttribI4uiv. These methods set integer vertex attributes with proper argument validation.

Key changes:

  • Added complete implementations for all four methods with argument count and type validation
  • Implemented support for both typed arrays (Int32Array/Uint32Array) and regular JavaScript arrays in the vector variants
  • Added minimum component count validation (4 components required) for array-based methods

Comment on lines +1207 to +1213
int index = args[0]->Int32Value(context).ToChecked();
int v0 = args[1]->Int32Value(context).ToChecked();
int v1 = args[2]->Int32Value(context).ToChecked();
int v2 = args[3]->Int32Value(context).ToChecked();
int v3 = args[4]->Int32Value(context).ToChecked();

handle()->vertexAttribI4i(index, v0, v1, v2, v3);
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS, usually 16 or 32 depending on implementation). Invalid index values could cause undefined behavior in the underlying OpenGL calls.

Copilot uses AI. Check for mistakes.
uint32_t v1 = args[2]->Uint32Value(context).ToChecked();
uint32_t v2 = args[3]->Uint32Value(context).ToChecked();
uint32_t v3 = args[4]->Uint32Value(context).ToChecked();

Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS). This matches the validation pattern needed in vertexAttribI4i.

Suggested change
// Validate index parameter
constexpr int MAX_VERTEX_ATTRIBS = 16; // WebGL2 minimum, replace with actual value if available
if (index < 0 || index >= MAX_VERTEX_ATTRIBS) {
isolate->ThrowException(Exception::RangeError(
String::NewFromUtf8(isolate, "vertexAttribI4ui: index out of range").ToLocalChecked()));
return;
}

Copilot uses AI. Check for mistakes.
return;
}

int index = args[0]->Int32Value(context).ToChecked();
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS). This validation is missing across all four methods.

Suggested change
int index = args[0]->Int32Value(context).ToChecked();
int index = args[0]->Int32Value(context).ToChecked();
// Validate index is non-negative and within bounds
constexpr int MAX_VERTEX_ATTRIBS = 16; // WebGL spec minimum, adjust if available from implementation
if (index < 0 || index >= MAX_VERTEX_ATTRIBS)
{
isolate->ThrowException(Exception::TypeError(
MakeMethodError(isolate, "vertexAttribI4iv", "index out of range")));
return;
}

Copilot uses AI. Check for mistakes.
return;
}

handle()->vertexAttribI4iv(index, values);
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS). This validation is missing across all four methods.

Copilot uses AI. Check for mistakes.
return;
}

int index = args[0]->Int32Value(context).ToChecked();
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS). This validation is missing across all four methods.

Copilot uses AI. Check for mistakes.
return;
}

handle()->vertexAttribI4uiv(index, values);
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS). This validation is missing across all four methods.

Copilot uses AI. Check for mistakes.
@EndlessJour9527
Copy link
Collaborator Author

@copilot please support vertexAttrib[1234]f[v] https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/vertexAttrib

Copy link
Contributor

Copilot AI commented Oct 29, 2025

@EndlessJour9527 I've opened a new pull request, #400, to work on those changes. Once the pull request is ready, I'll request review from you.

@yorkie yorkie merged commit 9747672 into main Nov 3, 2025
2 checks passed
@yorkie yorkie deleted the feat/support-vertext-attribI4x branch November 3, 2025 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants